-
Notifications
You must be signed in to change notification settings - Fork 652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new protocol for ChannelHandler to get buffered bytes in the channel handler #2918
base: main
Are you sure you want to change the base?
Conversation
…ohnnzhou/swift-nio into channel_handler_buffered_bytes
Thanks so much for opening this @johnnzhou! cc @weissi @Joannis @FranzBusch who are all going to care about this API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I am happy to see an API added for this. I am a bit unsure if the word audit
is the right one here. Could we just drop that from all the APIs here? What do others think?
Hi @FranzBusch, thanks for this suggestion. I originally designed this functionality as a way for users to inspect the number of bytes buffered in the channel handlers. Perhaps How about we use the word What do you think? |
Would this PR also (intend to) support child channels, such as in HTTP/2. |
Do we need count even here? What about
Depends what you expect. Every |
I'm generally ok with
My original plan is to implement this protocol for channel handlers provided by NIO. But maybe we need to make separate PRs for each handler? |
I understand where you are coming from. I personally still prefer that name but I would like to hear what @Lukasa @glbrntt or @weissi think.
We should definitely separate the API additions in this PR from the adoption. |
I'm happy for us to go with |
Sounds good. I will make the change accordingly. I will add more test cases in other commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting closer. Thanks for the work. I left some comments inline
Add new protocol to get buffered bytes from
ChannelHandler
s andChannelPipeline
API to query the buffered bytes fromChannelHandler
sMotivation:
In #2849, a new
ChannelOption
is introduced to retrieve the number of buffered outbound bytes in aChannel
. However, this solution does not account for bytes that may be buffered within individualChannelHandler
s. This PR builds on #2849 by adding functionality to audit buffered bytes residing inChannelHandler
s and exposing this information through newChannelPipeline
APIs.Modifications:
ChannelHandler
to audit buffered bytes for inbound and outbound.NIOOutboundByteBufferingChannelHandler
NIOInboundByteBufferingChannelHandler
ChannelPipeline
APIsResult:
Users can now easily query the amount of bytes buffered in
ChannelHandler
s using the newChannelPipeline
APIs, enhancing the visibility ofChannelHandler
performance.